Drop of "gist_geometry_ops" broke a few clients

[strk]

Brushtyler, on my left right now, had "gist_geometry_ops" use in his QGis plugin.

Indepentently, some geo-django module (on my right, with elpaso) uses "gist_geometry_ops".

Both fail against PostGIS-2.0.0SVN.

Looking at the code, we changed the name of it to "gist_geometry_ops_2d" to have space for "gist_geometry_ops_nd".

Is it worth it ?

Note that neither brushtyler, nor probably the geo-django people did anything weird, but probably just followed the PostGIS documentation (containing the instruction of using "gist_geometry_ops" when creating indices).

So, this is yet another compatibility issue to deal with.

Beside, I'm not sure what happens on incremental upgrades (if both geometry ops are left dangling in the DB, with possibly horrible consequences).

[strk]

upstream link to the django side (not filed by me!): ​https://code.djangoproject.com/ticket/16455

[robe]

Yah tell me about it. It broke osm2pgsql too which I noted in this ticket: http://trac.osgeo.org/postgis/ticket/1183 and remember it broke your topology code early on. Yes you were the first victim and it broke our shp2pgsql loader which we fixed.

But strk, as much as you would think I would be against Paul's change and be on your side, I am not. I welcome the clarity and having both and ops_2d and a regular ops does not make sense in my opinion and I certainly don't want to get rid of the 2d otherwise how will new people know that it is 2d and not nd or an all purpose guess what index to use based on your data. As I said the easy fix is just get rid of those stupid qualifiers in the code if you expect 2d. I know it requires 3rd party developers changing their code, but I honestly never understood why we encouraged people putting that in there anyway. I know its a tough call, but at least the change required would work for both earlier PostGIS and 2.0.

We can check on the db restore issue and see if there are any. I haven't noticed any in databases I restored from 1.5, but that might be because non of my tables had explicit create in them which is interesting since a lot were loaded with shp2pgsql which apparently had gist_geometry_ops hardcoded in there. Indexes all came back fine without the gist_geometry_ops.

[pramsey]

Actually I'm on the side of changing it back. When I changed it for clarity I didn't realize there were backward breaking consequences.

[robe]

well fight amongst yourselves, I fear changing it back at this late stage of the game will cause more damage since a lot of software has already changed their code to compensate

[strk]

Changed from "gist_geometry_ops" to "gist_geometry_ops_2d" ? Are we advertising existance of that ? We should just incourage NOT specifying the opname.

I don't care about those who "compensated" as 2.0 is not out yet and if you play with it you're strong enough to deal with troubles.

Of course it should be checked what happens exactly when you rename, and update the SO in a living database…

[robe]

strk — I'm all for not encouraging it. In the docs I changed , I didn't see any mention of ops or ops_2d. I added the use of ops_nd. That said haven't finished going thru the docs.

I'm not so much concerned about those who have compensated, as having to rip out a lot of code (all the ops have to change, my upgrade process is screwed up etc. - so I guess when I say those I mean ME). That said I think more people are annoyed about indecisiveness than any particular decision we make, as they've already started their development to be ready when we are.

We all prefer the new naming since its clearer, it will make obvious all those software trying to create tables with the old antiquated name which we'll desperately want to get rid of in the future, we've already sewn the seed, let's not pull out the plant so late in the game (I've done a lot of restores of databases on 2.0 I've had for ages and haven't run into an issue with this). Speaking of indecisiveness, can we please decide on what we will have for unknown, that is irritating people too — not what we will decide, but when. I'll put a vote on that on the table. At this point I don't care so much just that we come to a conclusion.

[strk]

Too late to go back, tagging this as "history" to remember to put in release notes.

[springmeyer]

as a note to other users that hit this problem, and find this ticket, it appears possible to patch postgis 2.0 for back compatibility here.

You can add to your db the below sql to re-implement gist_geometry_ops as identical to the new name of gist_geometry_op_2d.

The reason I am doing this is because osm2pgsql (while updated now to work with postgis 2.0) has proved very difficult to get compiled and working on windows (​https://github.com/springmeyer/osm2pgsql). So, for windows users I'm forging ahead with a method to get a 2 year old osm2pgsql working (the last available windows binary) with postgis 2.0 on windows, and this was the last piece of the puzzle after adding back srid 900913 and loading the legacy.sql.

Happy to hear from the devs if this shim is a terrible idea and there might be better ways to alias this.

CREATE OPERATOR CLASS gist_geometry_ops
	FOR TYPE geometry USING GIST AS
	STORAGE box2df,
	OPERATOR        1        <<  ,
	OPERATOR        2        &<	 ,
	OPERATOR        3        &&  ,
	OPERATOR        4        &>	 ,
	OPERATOR        5        >>	 ,
	OPERATOR        6        ~=	 ,
	OPERATOR        7        ~	 ,
	OPERATOR        8        @	 ,
	OPERATOR        9        &<| ,
	OPERATOR        10       <<| ,
	OPERATOR        11       |>> ,
	OPERATOR        12       |&> ,

	OPERATOR        13       <-> FOR ORDER BY pg_catalog.float_ops,
	OPERATOR        14       <#> FOR ORDER BY pg_catalog.float_ops,
	FUNCTION        8        geometry_gist_distance_2d (internal, geometry, int4),

	FUNCTION        1        geometry_gist_consistent_2d (internal, geometry, int4),
	FUNCTION        2        geometry_gist_union_2d (bytea, internal),
	FUNCTION        3        geometry_gist_compress_2d (internal),
	FUNCTION        4        geometry_gist_decompress_2d (internal),
	FUNCTION        5        geometry_gist_penalty_2d (internal, internal, internal),
	FUNCTION        6        geometry_gist_picksplit_2d (internal, internal),
	FUNCTION        7        geometry_gist_same_2d (geom1 geometry, geom2 geometry, internal);

[robe]

Dane,

Not that I would be of much, help, but I can try. What sort of problems are you running into compiling the latest osm2pgsql code base on windows? Admittedly I've never tried compiling osm2pgsql on windows — just always used your binaries

BTW Paul fixed the srid missing entry for 2.0 in #1805 so hopefully shouldn't be necessary for users to have to load that.

[springmeyer]

Regina,

osm2pgsql depends on a number of gnu extensions to c and recently started using os.fork/wait. So, basically my first attempt to try to get it working with msvc was not pretty. I may abort that effort and try msys next.

[robe]

Paul,

What's your feeling about providing this as part of legacy. I'm tempted to just have it as a separate file like what Mapserver does here:

​https://github.com/mapserver/mapserver/wiki/RenderingOsmDataWindows

Then just documenting the issue in our manual. My reason for not wanting to include in the core legacy, is I fear it will bite us later on since people following examples will accidentally start using it. Right now restoring data from 1.5 backup there is no issue, because since it was the default index option, the backups don't even have this gist_geometry_ops on load even if people chose it, but if people start using it globally then we'll have issues since they'll be forever stuck in legacy mode.

[pramsey]

See no evil, hear no evil, smell no evil. Document the hell out of it, but don't include it in legacy or anything. Make a gis.stackexchange entry and fill it full of good search keywords to bring people to the answer easily. Something.

[robe]

Okay I'll put in the FAQ somewhere. Already got 3 people whining about this when they read my osm tutorial (i think i even noted it in there too though I'll have to double-check on that).

[robe]

For this how about we include a legacy gist script (not part of legacy or legacy_minor)

Then I'll put this in the install instructions (if you absolutely need this) and also the FAQ.

I just hate pointing people to scripts we have no control over.

[robe]

Done for 2.1 at r10712. Still need to backport to 2.0.2 before I can close this out.

[robe]

Backported to 2.0 at r10714, minor typo corrections at r10713.

This is a both a documentation and code amendment.